Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

City Dialog: ExpCol present units list #2428

Merged
merged 2 commits into from
Oct 31, 2024
Merged

City Dialog: ExpCol present units list #2428

merged 2 commits into from
Oct 31, 2024

Conversation

jwrober
Copy link
Collaborator

@jwrober jwrober commented Oct 30, 2024

Closes #2295

@jwrober jwrober requested a review from lmoureaux October 30, 2024 23:07
lmoureaux
lmoureaux reviewed Oct 30, 2024
View reviewed changes
Copy link
Contributor

@lmoureaux lmoureaux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small style comments. IMNSHO the conditions would be easier to read if the if and else cases were inverted

client/citydlg.cpp Outdated Show resolved Hide resolved
client/citydlg.cpp Outdated Show resolved Hide resolved
@jwrober
Copy link
Collaborator Author

jwrober commented Oct 30, 2024

Small style comments. IMNSHO the conditions would be easier to read if the if and else cases were inverted

so do the true condition first with else being false?

@lmoureaux
Copy link
Contributor

Small style comments. IMNSHO the conditions would be easier to read if the if and else cases were inverted

so do the true condition first with else being false?

Yes, to avoid having to think about the ! or == false. But it's eventually a matter of taste.

@jwrober jwrober merged commit aa9849d into longturn:master Oct 31, 2024
21 checks passed
@jwrober jwrober deleted the feature/expand-units-city branch October 31, 2024 19:06
hugomflavio
hugomflavio reviewed Nov 9, 2024
View reviewed changes
Copy link
Contributor

@hugomflavio hugomflavio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested, looks pretty cool!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hugomflavio hugomflavio hugomflavio left review comments

@lmoureaux lmoureaux lmoureaux approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add "expand" button to units panel in new City Dialog
3 participants
@jwrober @lmoureaux @hugomflavio